Skip to content

fix(paint): unbreak shadows, bound the opacity layer, make the banding fix real - #154

Merged
LeadcodeDev merged 1 commit into
chantier/audit-remediationfrom
fix/paint-shadow-layer-gradient
Aug 8, 2026
Merged

fix(paint): unbreak shadows, bound the opacity layer, make the banding fix real#154
LeadcodeDev merged 1 commit into
chantier/audit-remediationfrom
fix/paint-shadow-layer-gradient

Conversation

@LeadcodeDev

Copy link
Copy Markdown
Owner

Round 3 du chantier d'audit — lot passe de peinture, 7 constats vérifiés (+ 1 garde-fou que j'ai ajouté en vérifiant).

Constat Sévérité Symptôme
overflow: hidden vs ombre High Le clip était posé avant le dessin de l'ombre : le box-shadow extérieur du nœud était découpé par la boîte même hors de laquelle il est censé déborder.
backdrop-filter High Neutralisé dès que opacity < 1 sur le même nœud.
Padding des painters de feuille High Canvas translaté sur la border-box avec les insets remis à zéro : tous les painters de feuille ignoraient padding.
save_layer sans bounds Medium Une couche plein viewport par nœud. Sur un scénario de 60 frames : 42–60 s → 0,5 s.
Pourcentages de transform Medium Résolus contre max(width, height) sur les deux axes au lieu d'un axe par axe.
Fonds tuilés défilants Medium L'offset croît linéairement avec le temps sans borne, alors que les boucles de dessin ne débordent que d'une tuile → bande vide grandissante.
Mitigations de banding Medium Les deux étaient inertes. Voir ci-dessous.

Codeblock reste une exception délibérée

Le contrat Painter promet un canvas déjà translaté sur l'origine de la content-box. Codeblock lit style.padding lui-même et peint son fond depuis la border-box : honorer le contrat général pour lui aussi doublerait le padding. L'exception est maintenant documentée dans le code plutôt que subie.

Pourquoi les deux mitigations de banding ne faisaient rien

Les surfaces de rendu sont créées sans ColorSpace Skia (ImageInfo::new(..., None), scene.rs). Cela court-circuite toute conversion colorimétrique — donc :

  1. Baliser les couleurs du shader avec ColorSpace::new_srgb_linear() était un no-op silencieux : les couleurs restaient en sRGB gamma-encodé.
  2. Subdiviser un lerp déjà en espace sRGB est une identité mathématique : 17× plus de stops, zéro effet visuel.

subdivide_gradient_stops fait désormais l'aller-retour gamma lui-même sur des f32 — ce qui fonctionne quel que soit l'espace colorimétrique que la surface de destination finira par porter. L'alpha n'est pas gamma-encodé et garde un lerp linéaire.

Le garde-fou que le wrap rendait nécessaire

Trouvé en vérifiant le constat, pas rapporté par l'agent. La géométrie est périodique sur spacing et traverse un wrap sans changer ; le pulse des points ne l'est pas — c'est un sin de la position. Alimenté en coordonnées canvas-locales, le rayon et l'alpha de tous les points sautaient d'un coup, toutes les spacing / speed secondes (≈ 0,67 s pour spacing 40 / speed 60), d'environ 17 %.

Le pulse lit maintenant la piste de défilement non wrappée. Le test qui l'affirme vérifie aussi que la version naïve saute toujours — il ne peut donc pas devenir silencieusement vide.

Reste ouvert

Le wrap s'applique aussi aux trois presets non périodiques (gradient_shift, halo, concentric_circles), qui animent déjà leur propre mouvement en interne. Pour eux, il substitue un artefact borné à une disparition non bornée : strictement meilleur, pas encore juste. Suivi séparément.

Vérification

cargo test -p rustmotion -p rustmotion-core -p rustmotion-components sur cette branche seule : 153 + 117 + 6 + 3 + 5 + 215 + 3 tests, 0 échec.

…g fix real

Seven confirmed audit findings in the paint pass, plus one regression guard
for a side effect the scroll-wrap fix would otherwise have introduced.

- `overflow: hidden` erased the node's own outset `box-shadow`: the clip
  was installed before the shadow was drawn, so the shadow was clipped away
  by the very box it was supposed to sit outside.
- `backdrop-filter` was neutralised whenever `opacity < 1` on the same node.
- The opacity `save_layer` was allocated with no bounds — one full-viewport
  layer per node. On a 60-frame scenario this dominated everything else:
  42-60s down to 0.5s.
- Leaf painters were handed the border-box origin with the padding insets
  zeroed, so every leaf ignored `padding`. `Codeblock` stays a deliberate,
  now-documented exception: it reads `style.padding` itself and paints its
  own background from the border box, so honouring the general contract for
  it too would double-apply padding.
- Transform percentages were resolved against `max(width, height)` on both
  axes instead of per-axis.
- Scrolling tiled backgrounds walked out of frame: the offset grew linearly
  with time forever while the draw loops only overscan by one tile, so the
  pattern left a growing blank band. The offset now wraps into one tile
  period, and `draw_bg_grid_dots`'s x-loop overscans symmetrically like its
  y-loop already did (it started at 0, with no left margin).
- Both documented gradient banding mitigations were inert. The render
  surfaces are created with no Skia `ColorSpace`, which short-circuits the
  conversion, so tagging the shader's colors with `srgb_linear` was a
  silent no-op — and subdividing an already-sRGB lerp is a mathematical
  identity (17x the stops, zero visual effect). `subdivide_gradient_stops`
  now does the gamma round-trip itself on plain `f32`s, which works whatever
  color space the destination surface ends up carrying.

The wrap above needed one companion fix, found while verifying it rather
than reported: geometry is periodic on `spacing` and survives a wrap, but
the dot pulse is a `sin` of position and is not. Fed canvas-local
coordinates, every dot's radius and alpha stepped at once, every
`spacing / speed` seconds. The pulse now reads the unwrapped scroll track,
and the test asserting this also asserts that the naive version still
steps — so it cannot quietly become vacuous.

Tests: 153 + 117 + 6 + 3 + 5 + 215 + 3 pass on this branch alone.
@LeadcodeDev LeadcodeDev added the bug Something isn't working label Aug 8, 2026
@LeadcodeDev LeadcodeDev self-assigned this Aug 8, 2026
@LeadcodeDev
LeadcodeDev merged commit 7b95a15 into chantier/audit-remediation Aug 8, 2026
3 checks passed
LeadcodeDev added a commit that referenced this pull request Aug 10, 2026
…g fix real (#154)

Seven confirmed audit findings in the paint pass, plus one regression guard
for a side effect the scroll-wrap fix would otherwise have introduced.

- `overflow: hidden` erased the node's own outset `box-shadow`: the clip
  was installed before the shadow was drawn, so the shadow was clipped away
  by the very box it was supposed to sit outside.
- `backdrop-filter` was neutralised whenever `opacity < 1` on the same node.
- The opacity `save_layer` was allocated with no bounds — one full-viewport
  layer per node. On a 60-frame scenario this dominated everything else:
  42-60s down to 0.5s.
- Leaf painters were handed the border-box origin with the padding insets
  zeroed, so every leaf ignored `padding`. `Codeblock` stays a deliberate,
  now-documented exception: it reads `style.padding` itself and paints its
  own background from the border box, so honouring the general contract for
  it too would double-apply padding.
- Transform percentages were resolved against `max(width, height)` on both
  axes instead of per-axis.
- Scrolling tiled backgrounds walked out of frame: the offset grew linearly
  with time forever while the draw loops only overscan by one tile, so the
  pattern left a growing blank band. The offset now wraps into one tile
  period, and `draw_bg_grid_dots`'s x-loop overscans symmetrically like its
  y-loop already did (it started at 0, with no left margin).
- Both documented gradient banding mitigations were inert. The render
  surfaces are created with no Skia `ColorSpace`, which short-circuits the
  conversion, so tagging the shader's colors with `srgb_linear` was a
  silent no-op — and subdividing an already-sRGB lerp is a mathematical
  identity (17x the stops, zero visual effect). `subdivide_gradient_stops`
  now does the gamma round-trip itself on plain `f32`s, which works whatever
  color space the destination surface ends up carrying.

The wrap above needed one companion fix, found while verifying it rather
than reported: geometry is periodic on `spacing` and survives a wrap, but
the dot pulse is a `sin` of position and is not. Fed canvas-local
coordinates, every dot's radius and alpha stepped at once, every
`spacing / speed` seconds. The pulse now reads the unwrapped scroll track,
and the test asserting this also asserts that the naive version still
steps — so it cannot quietly become vacuous.

Tests: 153 + 117 + 6 + 3 + 5 + 215 + 3 pass on this branch alone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant